Skip to content

Ensure logging is configured on remote task workers#21379

Open
devin-ai-integration[bot] wants to merge 4 commits intomainfrom
devin/OSS-6581-1774992663
Open

Ensure logging is configured on remote task workers#21379
devin-ai-integration[bot] wants to merge 4 commits intomainfrom
devin/OSS-6581-1774992663

Conversation

@devin-ai-integration
Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration bot commented Mar 31, 2026

When tasks run on remote Dask or Ray workers via a serialized context, the worker process may never have called setup_logging(). This is because setup_logging() lives in prefect.main, which is lazily imported via prefect.__getattr__ — remote workers that don't access names like prefect.flow or prefect.task never trigger that import. As a result, the APILogHandler is not attached to Prefect loggers and task run logs are silently lost.

Closes #18082
Closes #10829

Changes

New API in prefect.logging.configuration:

  • ensure_logging_setup() — idempotent function that calls setup_logging() only if PROCESS_LOGGING_CONFIG has not been populated. This keeps the internal state check encapsulated rather than leaking it to callers.
  • is_logging_configured() — simple query for whether logging has been set up in this process.

Task engine (SyncTaskRunEngine / AsyncTaskRunEngine):

  • In initialize_run(), after entering hydrated_context, call ensure_logging_setup() when self.context is not None (remote execution signal). For local task runs, PROCESS_LOGGING_CONFIG is already populated via the normal prefect.main import, so the call is a no-op.

Notes for reviewers

  • self.context is not None is used to gate the call, but local ThreadPool runs also set context (with cancel_event). This is fine because ensure_logging_setup() short-circuits via the PROCESS_LOGGING_CONFIG check — zero overhead for local runs.
  • is_logging_configured() is added and tested but not called by the implementation itself. It's exposed as public API for external consumers.
  • No integration-level test exercises the full path (task engine with serialized context on a remote Dask/Ray worker → logging setup). The unit tests cover the new configuration functions in isolation using the existing dictConfigMock fixture pattern.

Checklist

  • This pull request references any related issue by including "closes <link to issue>"
  • If this pull request adds new functionality, it includes unit tests that cover the changes
  • If this pull request removes docs files, it includes redirect settings in mint.json.
  • If this pull request adds functions or classes, it includes helpful docstrings.

Link to Devin session: https://app.devin.ai/sessions/c7590302b0bc40df9fba83c32ba15f91
Requested by: @desertaxle

When tasks execute on remote Dask/Ray workers via a serialized context,
the worker process may never have called setup_logging(), so the
APILogHandler is missing from Prefect loggers and task logs are silently
lost.

Add ensure_logging_setup() API in logging/configuration.py that
encapsulates the PROCESS_LOGGING_CONFIG check and only calls
setup_logging() if needed. Call it from both SyncTaskRunEngine and
AsyncTaskRunEngine initialize_run() when a serialized context is
present. Flush APILogHandler in the finally block to ensure logs are
sent before the task result is returned.

Closes #18082

Co-authored-by: Alex Streed <desertaxle@users.noreply.github.com>
Co-Authored-By: alex.s <ajstreed1@gmail.com>
@devin-ai-integration
Copy link
Copy Markdown
Contributor Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

@github-actions github-actions bot added bug Something isn't working integrations Related to integrations with other services labels Mar 31, 2026
@codspeed-hq
Copy link
Copy Markdown

codspeed-hq bot commented Mar 31, 2026

Merging this PR will not alter performance

✅ 2 untouched benchmarks


Comparing devin/OSS-6581-1774992663 (f0eee82) with main (42ae6fd)

Open in CodSpeed

The flush was running on all task runs (including local ThreadPool runs
where self.context is set to {cancel_event: ...}), disrupting the
EventsWorker service. The core fix (ensure_logging_setup) is sufficient
for the remote worker logging issue.

Co-authored-by: Alexander Streed <desertaxle@users.noreply.github.com>
Co-Authored-By: alex.s <ajstreed1@gmail.com>
…ve is_logging_configured

- Moved ensure_logging_setup() call from task_engine.py into
  hydrated_context() in context.py, so it runs automatically when
  a serialized context is provided (remote execution)
- Removed is_logging_configured() since it was only used in tests
- Removed ensure_logging_setup import/calls from task_engine.py

Co-authored-by: Alexander Streed <desertaxle@users.noreply.github.com>
Co-Authored-By: alex.s <ajstreed1@gmail.com>

with ExitStack() as stack:
if serialized_context:
from prefect.logging.configuration import ensure_logging_setup
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't defer this import unless it's absolutely necessary.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved to top-level import in f0eee82. Verified no circular import issue.

Co-authored-by: Alexander Streed <desertaxle@users.noreply.github.com>
Co-Authored-By: alex.s <ajstreed1@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working integrations Related to integrations with other services

Projects

None yet

1 participant